Skip to content

fix: Remove checks for unit zooms and symmetric rotations in template warp #3376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 10, 2024
Merged

fix: Remove checks for unit zooms and symmetric rotations in template warp #3376

merged 2 commits into from
Oct 10, 2024

Conversation

@effigies
Copy link
Member

@effigies effigies commented Oct 9, 2024

A user with a custom template is generating warps that have spacing [1.5 1.5 1.5]: https://neurostars.org/t/use-custom-dartel-template-in-fmriprep/30670/8

The check for unit spacing was mostly intended to avoid weird behavior where we might generate a bad affine. The interpretation of isotropic spacing is straightforward, so I think we can relax this. Creating a docker image to allow for testing.

@effigies effigies changed the title fix: Accept warps with isotropic spacing fix: Accept warps with isotropic spacing during resampling Oct 9, 2024
@codecov
Copy link

codecov bot commented Oct 9, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.77%. Comparing base (08933bf) to head (1607996).
Report is 13 commits behind head on maint/24.1.x.

Additional details and impacted files 
@@               Coverage Diff                @@
##           maint/24.1.x    #3376      +/-   ##
================================================
+ Coverage         71.70%   71.77%   +0.06%     
================================================
  Files                57       57              
  Lines              4259     4255       -4     
  Branches            640      459     -181     
================================================
  Hits               3054     3054              
+ Misses             1090     1086       -4     
  Partials            115      115

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@effigies effigies changed the title fix: Accept warps with isotropic spacing during resampling fix: Remove checks for unit zooms and symmetric rotations in template warp Oct 10, 2024
@effigies
Copy link
Member Author

I resampled the MNI T1w image to one with a shape of (193, 192, 229) and an affine of:

[[   0.     1.1    0.  -105.6]
 [   0.     0.    -1.2  118.8]
 [   1.     0.     0.   -78. ]
 [   0.     0.     0.     1. ]]

After registering the original image to the resampled image with antsRegistrationSyNQuick.sh (modified to produce a composite .h5 output), I was able to load it and verify that the reference it generated had the same shape and affine using the method we have here. I'm comfortable removing these checks.

That method is also reproduced in nitransforms 24+ (verified again), so we can move to that in 24.2.

@effigies effigies merged commit daefc6c into maint/24.1.x Oct 10, 2024
12 checks passed
@effigies effigies deleted the docker/isotropic-warps branch October 10, 2024 01:01
effigies added a commit that referenced this pull request Oct 12, 2024
@effigies
Merge tag '24.1.1'
1d6ac2b 
24.1.1 (October 10, 2024)

Bug fix release in the 24.1.x series.

Precomputed functional derivatives were not being correcly detected,
and a couple fixes for rare issues.

  * FIX: Remove checks for unit zooms and symmetric rotations in template warp (#3376)
  * FIX: Stop excluding FS minc_modify_header used during fallback registration (#3372)
  * FIX: Repair search for precomputed bold references (#3370)
  * FIX: Repair search for precomputed transforms (#3369)
@effigies effigies added this to the 24.1.1 milestone Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

24.1.1

Development

Successfully merging this pull request may close these issues.

2 participants

@effigies